-
Notifications
You must be signed in to change notification settings - Fork 584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use lychee cache #6161
Use lychee cache #6161
Conversation
Note that the check needs to succeed once for the cache to be stored (lychee doesn't update the cache if there's a failure anyway). |
@dmathieu, this does not look like fixing the following issue:
However, I am not against merging it as I think it is good to have the same CI configuration. |
The check needs to run successfully once (we can trigger it at a moment of low activity) to trigger the cache. |
If we can't ever run the check once, we'd either need to figure out a way to split the check (I don't think lychee can do that), or to add an auth token to the GitHub requests so the rate limit is increased. Alternatively, lycheeverse/lychee#1403 would allow us to update the cache with only the successful checks, making failures retry only what didn't succeed previously. That PR is stalled, and I haven't been able to figure out its issue so far. |
I think this is the way. We just need to make sure to limit the permissions for My guess is that it just needs Reference: https://github.com/lycheeverse/lychee/blob/master/docs/TROUBLESHOOTING.md#github-rate-limiting |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6161 +/- ##
=====================================
Coverage 66.8% 66.8%
=====================================
Files 206 206
Lines 13211 13211
=====================================
+ Hits 8833 8835 +2
+ Misses 4111 4110 -1
+ Partials 267 266 -1 |
This sets up a github token to authenticate requests made by lychee, so GitHub doesn't rate limit us so easily. Related: open-telemetry/opentelemetry-go-contrib#6161
A few months ago, we enabled lychee cache in the SDK repo, to prevent rate limit issues, but forgot to enable it here.
open-telemetry/opentelemetry-go#5160